allow reading uid from ContainerUser to set the Tar entry uid #49770
Conversation
github-actions
bot
added
the
Area-Containers
There was a problem hiding this comment.
Pull Request Overview
This PR enables reading the ContainerUser property to set the UID on tar entries when creating container layers. The implementation supports both numeric user IDs and the special "root" user name, applying the appropriate UID (0 for root) to all files and directories in the container layer.
- Added
TryParseUserIdmethod to parse container user strings into numeric UIDs - Modified
Layer.FromDirectoryto accept and apply user ID to tar entries - Updated both containerization entry points to use the parsed user ID
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
ContainerBuilder.cs |
Added TryParseUserId method and updated containerization to use parsed user ID |
CreateNewImage.cs |
Updated to parse and pass user ID to layer creation |
Layer.cs |
Modified to accept user ID parameter and apply it to tar entries |
LayerEndToEndTests.cs |
Added test to verify user ID is correctly applied to tar entries |
| var fileStream = File.OpenRead(file.FullName); | ||
| entry = new(TarEntryType.RegularFile, containerPath, entryAttributes) | ||
| { | ||
| Mode = mode, | ||
| DataStream = fileStream | ||
| DataStream = fileStream, | ||
| }; |
There was a problem hiding this comment.
The using statement was removed from the FileStream, but the manual disposal at line 182 only happens for entries with DataStream. This creates a resource leak if an exception occurs between opening the stream and writing the entry.
Copilot uses AI. Check for mistakes.
| using TransientTestFolder folder = new(); | ||
|
|
||
| string testFilePath = Path.Join(folder.Path, "TestFile.txt"); | ||
| string testString = $"Test content for {nameof(SingleFileInFolder)}"; |
There was a problem hiding this comment.
The nameof reference is incorrect - it should be {nameof(UserIdIsAppliedToFiles)} since that's the current test method name, not SingleFileInFolder.
| string testString = $"Test content for {nameof(SingleFileInFolder)}"; | |
| string testString = $"Test content for {nameof(UserIdIsAppliedToFiles)}"; |
Copilot uses AI. Check for mistakes.
baronfel
force-pushed
the
containers-flow-userid
branch
from
c6f140e to
00034f2
Compare
Fixes dotnet/sdk-container-builds#639
We read ContainerUser and, if it is an int or the well-known user name
root, apply that value as theUidon the TarEntries that we create.One thing that Docker does better here is allowing Groups, as well as user and group values that aren't integers (presumably there's some kind of lookup that happens?).